Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search Block: Add rounded block style #27220

Closed
wants to merge 1 commit into from

Conversation

aaronrobertshaw
Copy link
Contributor

Description

Adds a new rounded block style for the search block. This will add a border radius to the search block including adding adjusted border radii to the input and button when the button is placed "inside".

This is an alternate approach to adding border radius block support as suggested in #25791 (comment)

Note: This uses a CSS variable for the border radius value. This allows;

  • the value to be inherited and used with calc() for inner elements
  • the user to define the border radius variable within different scopes if it makes sense for different search blocks to have different radius values
  • the user to only have to set a single CSS variable rather than override multiple styles

How has this been tested?

Manually.

  1. Add a search block to a post
  2. Select the rounded block style from the sidebar
  3. Switch between different button positions via the block toolbar option and confirm rounded styles are applied
  4. Check the frontend also gets correct styles

Screenshots

SearchRoundedStyle

Types of changes

New feature.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@aaronrobertshaw aaronrobertshaw added [Type] Enhancement A suggestion for improvement. [Block] Search Affects the Search Block - used to display a search field labels Nov 24, 2020
@aaronrobertshaw aaronrobertshaw self-assigned this Nov 24, 2020
@aaronrobertshaw aaronrobertshaw force-pushed the add/search-border-radius-block-style branch from 39a7e2d to 25dc4b2 Compare November 24, 2020 10:14
@github-actions
Copy link

Size Change: +135 B (0%)

Total Size: 1.2 MB

Filename Size Change
build/block-library/index.js 148 kB +8 B (0%)
build/block-library/style-rtl.css 8.29 kB +64 B (0%)
build/block-library/style.css 8.29 kB +63 B (0%)
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/annotations/index.js 3.8 kB 0 B
build/api-fetch/index.js 3.42 kB 0 B
build/autop/index.js 2.84 kB 0 B
build/blob/index.js 664 B 0 B
build/block-directory/index.js 8.71 kB 0 B
build/block-directory/style-rtl.css 943 B 0 B
build/block-directory/style.css 942 B 0 B
build/block-editor/index.js 133 kB 0 B
build/block-editor/style-rtl.css 11.3 kB 0 B
build/block-editor/style.css 11.3 kB 0 B
build/block-library/editor-rtl.css 8.96 kB 0 B
build/block-library/editor.css 8.96 kB 0 B
build/block-library/theme-rtl.css 792 B 0 B
build/block-library/theme.css 793 B 0 B
build/block-serialization-default-parser/index.js 1.87 kB 0 B
build/block-serialization-spec-parser/index.js 3.06 kB 0 B
build/blocks/index.js 48.1 kB 0 B
build/components/index.js 172 kB 0 B
build/components/style-rtl.css 15.3 kB 0 B
build/components/style.css 15.3 kB 0 B
build/compose/index.js 9.95 kB 0 B
build/core-data/index.js 14.8 kB 0 B
build/data-controls/index.js 827 B 0 B
build/data/index.js 8.8 kB 0 B
build/date/index.js 11.2 kB 0 B
build/deprecated/index.js 768 B 0 B
build/dom-ready/index.js 571 B 0 B
build/dom/index.js 4.92 kB 0 B
build/edit-navigation/index.js 11.2 kB 0 B
build/edit-navigation/style-rtl.css 881 B 0 B
build/edit-navigation/style.css 885 B 0 B
build/edit-post/index.js 306 kB 0 B
build/edit-post/style-rtl.css 6.45 kB 0 B
build/edit-post/style.css 6.44 kB 0 B
build/edit-site/index.js 23.6 kB 0 B
build/edit-site/style-rtl.css 3.86 kB 0 B
build/edit-site/style.css 3.86 kB 0 B
build/edit-widgets/index.js 26.4 kB 0 B
build/edit-widgets/style-rtl.css 3.13 kB 0 B
build/edit-widgets/style.css 3.13 kB 0 B
build/editor/editor-styles-rtl.css 476 B 0 B
build/editor/editor-styles.css 478 B 0 B
build/editor/index.js 43.3 kB 0 B
build/editor/style-rtl.css 3.85 kB 0 B
build/editor/style.css 3.85 kB 0 B
build/element/index.js 4.62 kB 0 B
build/escape-html/index.js 735 B 0 B
build/format-library/index.js 6.86 kB 0 B
build/format-library/style-rtl.css 547 B 0 B
build/format-library/style.css 548 B 0 B
build/hooks/index.js 2.27 kB 0 B
build/html-entities/index.js 623 B 0 B
build/i18n/index.js 3.57 kB 0 B
build/is-shallow-equal/index.js 698 B 0 B
build/keyboard-shortcuts/index.js 2.54 kB 0 B
build/keycodes/index.js 1.94 kB 0 B
build/list-reusable-blocks/index.js 3.1 kB 0 B
build/list-reusable-blocks/style-rtl.css 476 B 0 B
build/list-reusable-blocks/style.css 476 B 0 B
build/media-utils/index.js 5.32 kB 0 B
build/notices/index.js 1.81 kB 0 B
build/nux/index.js 3.42 kB 0 B
build/nux/style-rtl.css 671 B 0 B
build/nux/style.css 668 B 0 B
build/plugins/index.js 2.56 kB 0 B
build/primitives/index.js 1.43 kB 0 B
build/priority-queue/index.js 790 B 0 B
build/redux-routine/index.js 2.84 kB 0 B
build/reusable-blocks/index.js 2.92 kB 0 B
build/rich-text/index.js 13.4 kB 0 B
build/server-side-render/index.js 2.77 kB 0 B
build/shortcode/index.js 1.69 kB 0 B
build/token-list/index.js 1.27 kB 0 B
build/url/index.js 4.05 kB 0 B
build/viewport/index.js 1.86 kB 0 B
build/warning/index.js 1.14 kB 0 B
build/wordcount/index.js 1.22 kB 0 B

compressed-size-action

@glendaviesnz
Copy link
Contributor

glendaviesnz commented Nov 30, 2020

This tested well for me, my only question was whether there should be some extra padding on left of placeholder text when rounded chosen as text looks slightly odd at the edge of the radius, eg.

Instead of this:
Screen Shot 2020-11-30 at 5 01 20 PM

does this look better?:
Screen Shot 2020-11-30 at 5 01 07 PM

Might just be a personal opinion thing so not critical I don't think

@aaronrobertshaw
Copy link
Contributor Author

This tested well for me, my only question was whether there should be some extra padding on left of placeholder text when rounded chosen as text looks slightly odd at the edge of the radius

Agreed, I'll make that update.

I'm also still waiting to see if we go down this path, a separate border radius control to allow custom radius values or the full block support approach. #25791 (comment)

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, and testing it everything seems to be OK.
If we decide to go with this implementation instead of a separate border-radius control this looks OK to merge 👍

Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the direction we should go in because designers should have more fine grained control of border radius than this in order to create interesting patterns. Other examples of this need are here and here.

For now I think we should merge the block support for border radius and hold off on this.

@aaronrobertshaw
Copy link
Contributor Author

Closing this now #25791 has been merged.

There is a new PR to opt into the border radius support here #27664. It is simply the extracted search block updates from the border radius support PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Search Affects the Search Block - used to display a search field [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants